Skip to content

fix(storage): 使用文件级锁处理 content/write#2064

Merged
qin-ctx merged 1 commit into
mainfrom
fix/content-write-exact-lock
May 15, 2026
Merged

fix(storage): 使用文件级锁处理 content/write#2064
qin-ctx merged 1 commit into
mainfrom
fix/content-write-exact-lock

Conversation

@qin-ctx
Copy link
Copy Markdown
Collaborator

@qin-ctx qin-ctx commented May 15, 2026

Description

修复 #2029 合入后遗漏的一处资源写入锁粒度问题：content/write 写入 viking://resources/... 下的普通文件时，不再对资源根目录持有 TreeLock，改为只对目标文件路径持有 ExactPathLock。

举例：

  • 修复前：viking://resources/docs/api1.md 和 viking://resources/docs/api2.md 都会竞争 viking://resources/docs 的树锁，第二个请求容易返回 resource is busy。
  • 修复后：两个请求分别锁 api1.md 和 api2.md，不同文件可以并发写入；同一个文件、或遇到上层树锁时仍然会互斥。

Related Issue

N/A

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • 将资源 content/write 的锁路径从 root_uri 改为当前写入的 uri，锁类型从 TreeLock 改为 ExactPathLock。
  • 写入并投递语义刷新消息后立即释放文件级锁，不再把该锁作为生命周期锁转交给后台语义任务。
  • 更新已有服务层单测断言，匹配写入完成后释放文件锁的预期行为。

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

未额外运行单测。这个分支只补回 #2029 之后遗漏的 content/write 文件级锁修复，并更新了已有测试的锁释放预期。

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

N/A

Additional Notes

这个修复只覆盖直接文件写入路径。目录级资源生命周期保护、后台语义刷新 coalesce、以及上层树锁冲突判断仍沿用 #2029 的实现。

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

2029 - Fully compliant

Compliant requirements:

  • Use ExactPathLock instead of TreeLock for content/write
  • Release lock after enqueuing semantic refresh instead of transferring
  • Update tests to match new lock behavior
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 90
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ensure lock release on BaseException

The lock may not be released if a BaseException (like asyncio.CancelledError) is
raised, since the except block only catches Exception. Move the lock release check
to the finally block to ensure cleanup even on cancellations or other
BaseExceptions.

openviking/storage/content_write.py [224-238]

 except Exception:
     if not semantic_enqueued and content_written:
         await self._rollback_direct_write(
             uri=uri,
             previous_content=previous_content,
             mode=mode,
             ctx=ctx,
             lock_handle=handle,
         )
+    raise
+finally:
     if not lock_released:
         await lock_manager.release(handle)
-    raise
-finally:
     if wait and telemetry_id:
         get_request_wait_tracker().cleanup(telemetry_id)
Suggestion importance[1-10]: 8

__

Why: Moving the lock release check to the finally block ensures the lock is released even if a BaseException (like asyncio.CancelledError) is raised, preventing potential deadlocks.

Medium

@qin-ctx qin-ctx merged commit f79f69d into main May 15, 2026
5 checks passed
@qin-ctx qin-ctx deleted the fix/content-write-exact-lock branch May 15, 2026 06:03
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fengluodb fengluodb fengluodb approved these changes

Assignees

No one assigned

Labels

Review effort 2/5

Projects

OpenViking project
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@qin-ctx @fengluodb